-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Widget scrolling plus #4701
base: develop
Are you sure you want to change the base?
Widget scrolling plus #4701
Conversation
widget/table.go
Outdated
@@ -344,6 +344,17 @@ func (t *Table) TouchCancel(*mobile.TouchEvent) { | |||
// Implements: fyne.Focusable | |||
func (t *Table) TypedKey(event *fyne.KeyEvent) { | |||
switch event.Name { | |||
case fyne.KeyHome: | |||
t.ScrollToTop() | |||
t.Refresh() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Refresh() calls here should not be necessary since the Scroll APIs already refresh
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You were partially right.
The default ScrollToTop/ScrollToBottom did not work correctly, a t.content.Refresh() was missing before t.finishScroll(), so I added it. Without it, it does scroll to top or bottom, but the table becomes blank.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry this has sat for so long. Something doesn't seem right though. Are you saying that the current "ScrollToTop" or "ScrollToBottom" don't work unless there is a Refresh call in the developers code?
@@ -541,6 +552,33 @@ func (t *Table) ScrollToTop() { | |||
t.finishScroll() | |||
} | |||
|
|||
// scrollByOnePage scrolls down or up by table height | |||
func (t *Table) scrollByOnePage(down bool) { | |||
if t.Length == nil || t.content == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may need to add some offset validation here - please test what happens if you try to keep scrolling when you're already at the top or bottom of the list
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not go out of bounds!
There is one bug in the tree widget, but its not introduced with this PR, it just became obvious after implementing it.
When you call tree.ScrollToBottom() twice, the last row's top border vanishes.
Other than that, there are no problems. Not sure if I should investigate it, since its not part of this PR.
I can share some example code (120 lines).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If its not related then open an issue - the example code will be useful there :)
@@ -274,6 +290,16 @@ func (l *List) ScrollToOffset(offset float32) { | |||
l.Refresh() | |||
} | |||
|
|||
// ScrollUpOnePage scrolls up one page (table height) | |||
func (l *List) ScrollUpOnePage() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure these need to be public APIs (you can do the equivalent by ScrollToOffset(l.GetScrollOffset() + l.Size().Height)
) but if they are going to be new public APIs they need the // Since: 2.5
designation
Description:
Fixes #4695
Checklist:
Where applicable: